Skip to content

docs: security contacts adr [CM-1325]#4375

Merged
mbani01 merged 4 commits into
mainfrom
docs/security-contacts-adr
Jul 22, 2026
Merged

docs: security contacts adr [CM-1325]#4375
mbani01 merged 4 commits into
mainfrom
docs/security-contacts-adr

Conversation

@mbani01

@mbani01 mbani01 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

This pull request adds a new architecture decision record (ADR-0009) that documents the design and implementation details for the security-contacts worker, which extracts, scores, and ingests security contact information for critical repositories. The ADR covers the tiered extraction hierarchy, confidence scoring, data reconciliation, GitHub API usage, write semantics, scheduling, and the on-demand ingestion path, as well as alternatives considered and associated risks. Additionally, the ADR is linked in the ADR index.

New ADR documenting security-contacts worker:

  • Added docs/adr/0009-security-contacts-worker.md to comprehensively describe the architecture and decision process for the security-contacts worker, including extraction tiers, scoring, reconciliation, and operational details.
  • Updated docs/adr/README.md to reference ADR-0009 in the ADR index.

mbani01 added 2 commits July 21, 2026 17:59
Signed-off-by: Mouad BANI <mouad-mb@outlook.com>
Signed-off-by: Mouad BANI <mouad-mb@outlook.com>
@mbani01
mbani01 requested a review from joanagmaia July 21, 2026 17:06
@mbani01 mbani01 self-assigned this Jul 21, 2026
Copilot AI review requested due to automatic review settings July 21, 2026 17:06
@cursor

cursor Bot commented Jul 21, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Documentation-only change with no application, schema, or deployment impact.

Overview
Adds ADR-0010 (docs/adr/0010-security-contacts-worker.md) as the consolidated record for the security-contacts sub-worker and registers it in the ADR index.

The ADR documents the accepted design (not new runtime code in this PR): Temporal daily batch ingestion plus an on-demand single-PURL workflow; tiered extractors (A/B/D); reconciliation, junk filtering, and [0,1] confidence scoring with bands; GitHub rate-limit gateway behavior; soft-delete upsert writes, partial-pass merge rules, and refresh cadence; handle verification and CDP email resolution; API semantics (securityContacts: null vs []); alternatives rejected; and operational risks/trade-offs.

Reviewed by Cursor Bugbot for commit 2f107e1. Bugbot is set up for automated code reviews on this repo. Configure here.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds ADR-0009 documenting the security-contacts worker architecture and links it from the ADR index.

Changes:

  • Documents extraction, scoring, persistence, scheduling, and reconciliation decisions.
  • Records alternatives, consequences, and risks.
  • Updates the ADR index.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File Description
docs/adr/0009-security-contacts-worker.md Adds the security-contacts ADR.
docs/adr/README.md Links ADR-0009.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +27 to +28
comes back empty, 24 h execution timeout across the chain), plus a separate **on-demand
single-purl workflow** invoked by the akrites API when it hits a never-evaluated repo. Each repo
| A4 | RFC 9116 `security.txt` on the project homepage | `securityTxt.ts` |
| B1 | `SECURITY.md` (root, `.github/`, `docs/`) | `securityMd.ts` |
| B2 | Registry manifests — npm, PyPI, Maven, Cargo, NuGet, RubyGems, Composer | `extractors/registry/` |
| D | Top-3 committers (last 90 days) + repo owner profile; static Go ecosystem fallback | `topCommitters.ts`, `repoOwner.ts`, `registry/go.ts` |
Comment on lines +182 to +184
- **Partial passes merge only**: when an extractor failed, the soft-delete step is skipped — a
source that wasn't consulted cannot wipe contacts it didn't see. Stale rows are cleaned on the
next fully-successful pass.
- Idempotent, self-advancing sweep: failed repos are marked attempted, failed chunks re-extract in
isolation, and re-runs converge instead of duplicating.
- The on-demand path fills coverage for non-critical purls lazily, exactly when the API needs them.
- Pure scoring/reconcile functions are unit-tested in isolation (`__tests__/`).
Comment on lines +278 to +280
- **Single-writer assumption** — `writeContacts` takes no lock; correctness relies on the Temporal
schedule (SKIP overlap) and heartbeat-based supersession keeping one writer per repo. A future
second caller (e.g. a backfill script) must respect this or add locking.
Comment on lines +62 to +65
- **Go packages get a static ecosystem fallback**: Go has no registry manifest carrying contact
metadata (the module proxy and pkg.go.dev expose none), so the Go fetcher makes no HTTP call and
always emits two tier-D `security-team` contacts per the Go security policy —
`security@golang.org` and the `https://g.co/vulnz` report form. At tier D they never outrank a
joanagmaia
joanagmaia previously approved these changes Jul 22, 2026

@joanagmaia joanagmaia left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a matter of updating the ADR id and good to merge

@@ -0,0 +1,283 @@
# ADR-0009: Security contacts — tiered extraction, confidence scoring, and Temporal batch ingestion

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Conflicting ADR with main

Signed-off-by: Mouad BANI <mouad-mb@outlook.com>
Copilot AI review requested due to automatic review settings July 22, 2026 16:18
Signed-off-by: Mouad BANI <mbani@contractor.linuxfoundation.org>
@mbani01
mbani01 merged commit 1c4fe98 into main Jul 22, 2026
7 checks passed
@mbani01
mbani01 deleted the docs/security-contacts-adr branch July 22, 2026 16:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (4)

docs/adr/0010-security-contacts-worker.md:43

  • Tier D is described as implemented, but none of the named extractor files exist and processBatch.ts:50-57 registers only A1 through B2. No code under security-contacts emits a tier-D contact. Either add the D extractors before accepting this ADR or clearly mark this tier and all dependent claims as proposed/unimplemented.
| D | Top-3 committers (last 90 days) + repo owner profile; static Go ecosystem fallback | `topCommitters.ts`, `repoOwner.ts`, `registry/go.ts` |

docs/adr/0010-security-contacts-worker.md:67

  • The documented Go fallback is not implemented: there is no registry/go.ts, no golang.org/g.co/vulnz contact source, and extractors/registry/index.ts:19-28 explicitly omits Go from FETCHERS. As written, Go repos can still receive an empty result, contrary to this accepted decision.
- **Go packages get a static ecosystem fallback**: Go has no registry manifest carrying contact
  metadata (the module proxy and pkg.go.dev expose none), so the Go fetcher makes no HTTP call and
  always emits two tier-D `security-team` contacts per the Go security policy —
  `security@golang.org` and the `https://g.co/vulnz` report form. At tier D they never outrank a
  real repo-level contact, and they don't suppress the committers/owner gate (which only checks
  for usable contacts above tier D), so a Go repo is never left with an empty answer.

docs/adr/0010-security-contacts-worker.md:207

  • The worker exposes this on-demand workflow, but no API code invokes it. Repository-wide references to ingestSecurityContactsForPurlWorkflow are limited to its definition and worker re-export; getPackage.ts:41-54 simply returns null when contacts have not been evaluated. Please wire the API cache-miss path to the packages Temporal client, or document that the caller integration is still pending rather than saying the request triggers ingestion.
On the read side, the API contract distinguishes **`securityContacts: null`** (repo never
evaluated — `contacts_last_refreshed IS NULL`, which is what triggers the on-demand ingest) from
**`[]`** (evaluated, nothing found — no re-ingest until the daily retry). `packageConfidence` is
derived at read time as the band of the highest contact score, not stored.

docs/adr/0010-security-contacts-worker.md:280

  • The single-writer assumption already has a second writer in this ADR: the on-demand workflow calls writeContacts independently of the scheduled batch. ScheduleOverlapPolicy.SKIP only serializes schedule actions; it does not prevent on-demand executions (or concurrent cache misses) from writing the same repo. Record and implement a per-repo serialization/locking strategy, or explicitly accept and analyze the concurrent-write behavior.
- **Single-writer assumption** — `writeContacts` takes no lock; correctness relies on the Temporal
  schedule (SKIP overlap) and heartbeat-based supersession keeping one writer per repo. A future
  second caller (e.g. a backfill script) must respect this or add locking.

Comment thread docs/adr/README.md
@@ -16,6 +16,7 @@ Use the `/adr` skill in Claude Code to record new ADRs or query past decisions.
| [ADR-0007](./0007-test-factory-primitives-and-defaults.md) | Test factory primitives and defaults | accepted | 2026-07-10 |
| [ADR-0008](./0008-how-we-write-unit-tests.md) | How we write unit tests | accepted | 2026-07-13 |
| [ADR-0009](./0009-packagist-worker-design-decisions.md) | Packagist worker — design decisions | accepted | 2026-07-13 |
| ---- | ------ | --------- |
| A1 | `SECURITY-INSIGHTS.yml` (root, `.github/`, `.gitlab/`) | `securityInsights.ts` |
| A2 | GitHub private vulnerability reporting status (authed) | `pvr.ts` |
| A3 | `SECURITY_CONTACTS` / `OWNERS` (k8s-style) | `securityContactsFile.ts` |
Copilot AI review requested due to automatic review settings July 22, 2026 16:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (3)

docs/adr/0010-security-contacts-worker.md:28

  • The on-demand workflow exists in the worker, but the Akrites API does not invoke it. Repository-wide references to ingestSecurityContactsForPurlWorkflow are limited to the workflow definition/export and this ADR, while getPackage.ts only converts a null refresh timestamp to securityContacts: null. Wire the cache-miss path through getPackagesTemporalClient() or document this as unimplemented rather than an accepted behavior.
comes back empty, 24 h execution timeout across the chain), plus a separate **on-demand
single-purl workflow** invoked by the akrites API when it hits a never-evaluated repo. Each repo

docs/adr/0010-security-contacts-worker.md:39

  • securityContactsFile.ts probes only the root SECURITY_CONTACTS path (PATH = 'SECURITY_CONTACTS' and a single contents request at lines 11 and 58-60); it does not discover an OWNERS file. Remove OWNERS from the implemented source list unless support is added.
| A3 | `SECURITY_CONTACTS` / `OWNERS` (k8s-style) | `securityContactsFile.ts` |

docs/adr/0010-security-contacts-worker.md:280

  • The current design already has a second writer: the API-triggerable on-demand workflow can process the same repo as the scheduled batch, and ScheduleOverlapPolicy.SKIP only coordinates runs of the schedule. Neither path takes a per-repo lock, so the asserted single-writer guarantee is not provided and concurrent soft-delete/upsert transactions can produce nondeterministic last-writer results. Treat the on-demand path as an existing concurrency risk and add per-repo serialization/locking rather than deferring this to a hypothetical future caller.
- **Single-writer assumption** — `writeContacts` takes no lock; correctness relies on the Temporal
  schedule (SKIP overlap) and heartbeat-based supersession keeping one writer per repo. A future
  second caller (e.g. a backfill script) must respect this or add locking.

@@ -0,0 +1,283 @@
# ADR-0010: Security contacts — tiered extraction, confidence scoring, and Temporal batch ingestion
**Status**: accepted
**Deciders**: Mouad BANI

_Consolidated ADR for the security-contacts worker — record further security-contacts decisions here rather than opening new ADRs._
Comment on lines +18 to +20
data is consumed by the public `/v1/akrites/packages/detail` endpoint, so it must carry enough
provenance and confidence signal for a downstream security team to trust — or discount — each
contact. The worker follows the ADR-0001 §Worker architecture pattern
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants